Skip to content

[iOS] Make font styling work when using specific font name on the new architecture#37109

Closed
j-piasecki wants to merge 1 commit into
react:mainfrom
j-piasecki:patch-1
Closed

[iOS] Make font styling work when using specific font name on the new architecture#37109
j-piasecki wants to merge 1 commit into
react:mainfrom
j-piasecki:patch-1

Conversation

@j-piasecki

Copy link
Copy Markdown
Contributor

Summary:

Currently, when fontFamily style is set to a specific font instead of a font family, that specific font is used to display the text on iOS when using the new architecture. This is different behavior to the old architecture, where the font family and font properties were extracted from the specified font and overridden if not provided by the user.

Changelog:

[IOS] [FIXED] - Make font resolution work when using specific font name on the new architecture

Test Plan:

You can verify the problem on a simple snippet:

import React from 'react';
import {SafeAreaView, Text} from 'react-native';

function App() {
  return (
    <SafeAreaView style={{flex: 1}}>
      <Text
        style={{
          fontFamily: 'Helvetica Light Oblique',
          fontWeight: 'bold',
          fontStyle: 'normal',
        }}>
        Some random text
      </Text>
    </SafeAreaView>
  );
}

export default App;
Here's before & after

Without changes from this PR:

With changes from this PR:

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 26, 2023
@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,625,130 +0
android hermes armeabi-v7a 7,937,983 +0
android hermes x86 9,112,076 +0
android hermes x86_64 8,966,887 +0
android jsc arm64-v8a 9,189,170 +0
android jsc armeabi-v7a 8,379,482 +0
android jsc x86 9,247,475 +0
android jsc x86_64 9,505,920 +0

Base commit: 2cda4f7
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@sammy-SC has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

sammy-SC pushed a commit to sammy-SC/react-native that referenced this pull request May 3, 2023
…tecture (react#37109)

Summary:
Currently, when `fontFamily` style is set to a specific font instead of a font family, [that specific font is used](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTFontUtils.mm#L126) to display the text on iOS when using the new architecture. This is different behavior to the old architecture, where the font family and [font properties were extracted from the specified](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/React/Views/RCTFont.mm#L450-L457) font and overridden if not provided by the user.

## Changelog:

[IOS] [FIXED] - Make font resolution work when using specific font name on the new architecture

Pull Request resolved: react#37109

Test Plan:
You can verify the problem on a simple snippet:
```jsx
import React from 'react';
import {SafeAreaView, Text} from 'react-native';

function App() {
  return (
    <SafeAreaView style={{flex: 1}}>
      <Text
        style={{
          fontFamily: 'Helvetica Light Oblique',
          fontWeight: 'bold',
          fontStyle: 'normal',
        }}>
        Some random text
      </Text>
    </SafeAreaView>
  );
}

export default App;
```

<details>
<summary>
Here's before & after
</summary>

Without changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618852-07cbe67c-f534-4b04-b760-828f4edef549.png" width=400 />

With changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618902-9e44a389-8f27-4ab0-95dc-e34ca781d2ed.png" width=400 />

</details>

Differential Revision: D45351185

Pulled By: sammy-SC

fbshipit-source-id: 09b17b75d58f2cfe82dc755424632ab8cc6da55e
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions Bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 25, 2023
@github-actions

github-actions Bot commented Nov 1, 2023

Copy link
Copy Markdown

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions Bot closed this Nov 1, 2023
facebook-github-bot pushed a commit that referenced this pull request Feb 7, 2024
…tecture (#37109)

Summary:
Currently, when `fontFamily` style is set to a specific font instead of a font family, [that specific font is used](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTFontUtils.mm#L126) to display the text on iOS when using the new architecture. This is different behavior to the old architecture, where the font family and [font properties were extracted from the specified](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/React/Views/RCTFont.mm#L450-L457) font and overridden if not provided by the user.

## Changelog:

[IOS] [FIXED] - Make font resolution work when using specific font name on the new architecture

Pull Request resolved: #37109

Test Plan:
You can verify the problem on a simple snippet:
```jsx
import React from 'react';
import {SafeAreaView, Text} from 'react-native';

function App() {
  return (
    <SafeAreaView style={{flex: 1}}>
      <Text
        style={{
          fontFamily: 'Helvetica Light Oblique',
          fontWeight: 'bold',
          fontStyle: 'normal',
        }}>
        Some random text
      </Text>
    </SafeAreaView>
  );
}

export default App;
```

<details>
<summary>
Here's before & after
</summary>

Without changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618852-07cbe67c-f534-4b04-b760-828f4edef549.png" width=400 />

With changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618902-9e44a389-8f27-4ab0-95dc-e34ca781d2ed.png" width=400 />

</details>

Reviewed By: NickGerleman

Differential Revision: D45351185

Pulled By: sammy-SC

fbshipit-source-id: 1f0a6a52a714ca4989775817d1fb53ae593143f8
@github-actions

github-actions Bot commented Feb 7, 2024

Copy link
Copy Markdown

This pull request was successfully merged by @j-piasecki in 6177408.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions Bot added the Merged This PR has been merged. label Feb 7, 2024
facebook-github-bot pushed a commit that referenced this pull request Feb 8, 2024
… new architecture" (#42919)

Summary:
Changelog:
[IOS] [FIXED] - backout #37109

Pull Request resolved: #42919

Original commit changeset: 1f0a6a52a714

Original Phabricator Diff: D45351185

Reviewed By: cipolleschi

Differential Revision: D53566506

fbshipit-source-id: e96813edfee4b54a7828ac02c3dc8aaffc83d6b7
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been reverted by f6b984d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: Text Merged This PR has been merged. Reverted Stale There has been a lack of activity on this issue and it may be closed soon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants